Skip to content

Conversation

klausler
Copy link
Contributor

@klausler klausler commented Sep 5, 2025

When a procedure name is declared EXTERNAL and then followed up with an explicit INTERFACE, the compiler emits an error when the procedure is a dummy procedure but only a warning otherwise. Since the EXTERNAL statement is harmless in both cases, accept the case of a dummy procedure as well for consistency.

Fixes #157162.

When a procedure name is declared EXTERNAL and then followed
up with an explicit INTERFACE, the compiler emits an error
when the procedure is a dummy procedure but only a warning
otherwise.  Since the EXTERNAL statement is harmless in both
cases, accept the case of a dummy procedure as well for consistency.

Fixes llvm#157162.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Sep 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2025

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

When a procedure name is declared EXTERNAL and then followed up with an explicit INTERFACE, the compiler emits an error when the procedure is a dummy procedure but only a warning otherwise. Since the EXTERNAL statement is harmless in both cases, accept the case of a dummy procedure as well for consistency.

Fixes #157162.


Full diff: https://github.com/llvm/llvm-project/pull/157191.diff

3 Files Affected:

  • (modified) flang/lib/Semantics/resolve-names.cpp (+8-2)
  • (modified) flang/lib/Semantics/symbol.cpp (+10-6)
  • (modified) flang/test/Semantics/resolve20.f90 (+8)
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 4720932780472..077bee930675e 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -646,12 +646,18 @@ class ScopeHandler : public ImplicitRulesVisitor {
     }
     if (symbol->CanReplaceDetails(details)) {
       // update the existing symbol
-      CheckDuplicatedAttrs(name, *symbol, attrs);
-      SetExplicitAttrs(*symbol, attrs);
       if constexpr (std::is_same_v<SubprogramDetails, D>) {
         // Dummy argument defined by explicit interface?
         details.set_isDummy(IsDummy(*symbol));
+        if (symbol->has<ProcEntityDetails>()) {
+          // Bare "EXTERNAL" dummy replaced with explicit INTERFACE
+          context().Warn(common::LanguageFeature::RedundantAttribute, name,
+              "Dummy argument '%s' was declared earlier as EXTERNAL"_warn_en_US,
+              name);
+        }
       }
+      CheckDuplicatedAttrs(name, *symbol, attrs);
+      SetExplicitAttrs(*symbol, attrs);
       symbol->set_details(std::move(details));
       return *symbol;
     } else if constexpr (std::is_same_v<UnknownDetails, D>) {
diff --git a/flang/lib/Semantics/symbol.cpp b/flang/lib/Semantics/symbol.cpp
index a6b402c48d4ff..ea7eeac80a2d9 100644
--- a/flang/lib/Semantics/symbol.cpp
+++ b/flang/lib/Semantics/symbol.cpp
@@ -330,8 +330,14 @@ bool Symbol::CanReplaceDetails(const Details &details) const {
         common::visitors{
             [](const UseErrorDetails &) { return true; },
             [&](const ObjectEntityDetails &) { return has<EntityDetails>(); },
-            [&](const ProcEntityDetails &) { return has<EntityDetails>(); },
+            [&](const ProcEntityDetails &x) { return has<EntityDetails>(); },
             [&](const SubprogramDetails &) {
+              if (const auto *oldProc{detailsIf<ProcEntityDetails>()}) {
+                // Can replace bare "EXTERNAL dummy" with explicit INTERFACE
+                return oldProc->isDummy() && !oldProc->procInterface() &&
+                    attrs().test(Attr::EXTERNAL) && !test(Flag::Function) &&
+                    !test(Flag::Subroutine);
+              }
               return has<SubprogramNameDetails>() || has<EntityDetails>();
             },
             [&](const DerivedTypeDetails &) {
@@ -339,14 +345,12 @@ bool Symbol::CanReplaceDetails(const Details &details) const {
               return derived && derived->isForwardReferenced();
             },
             [&](const UseDetails &x) {
-              const auto *use{this->detailsIf<UseDetails>()};
+              const auto *use{detailsIf<UseDetails>()};
               return use && use->symbol() == x.symbol();
             },
-            [&](const HostAssocDetails &) {
-              return this->has<HostAssocDetails>();
-            },
+            [&](const HostAssocDetails &) { return has<HostAssocDetails>(); },
             [&](const UserReductionDetails &) {
-              return this->has<UserReductionDetails>();
+              return has<UserReductionDetails>();
             },
             [](const auto &) { return false; },
         },
diff --git a/flang/test/Semantics/resolve20.f90 b/flang/test/Semantics/resolve20.f90
index 8b8d190206689..f1a1a30cc714e 100644
--- a/flang/test/Semantics/resolve20.f90
+++ b/flang/test/Semantics/resolve20.f90
@@ -89,4 +89,12 @@ subroutine test
     !ERROR: Abstract procedure interface 'f' may not be referenced
     x = f()
   end subroutine
+  subroutine baz(foo)
+    external foo
+    interface
+      !WARNING: Dummy argument 'foo' was declared earlier as EXTERNAL [-Wredundant-attribute]
+      subroutine foo(x)
+      end
+    end interface
+  end
 end module

Copy link
Collaborator

@kkwli kkwli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG. Thanks for the quick fix.

@kkwli
Copy link
Collaborator

kkwli commented Sep 8, 2025

Sorry, I miss this case. Should it be covered in this fix?

      interface
        subroutine g(f)
          external f
          interface
            subroutine f()
            end
          end interface
        end
      end interface
      end

Output:

error: Semantic errors in dd2.f90
./dd2.f90:5:24: error: 'f' is already declared in this scoping unit
              subroutine f()
                         ^
./dd2.f90:2:22: Previous declaration of 'f'
          subroutine g(f)
                       ^

@kkwli
Copy link
Collaborator

kkwli commented Sep 8, 2025

If f is not a dummy procedure, the warning is issue.

      interface
        subroutine g()
          external f
          interface
            subroutine f()
            end
          end interface
        end
      end interface
      end

Output:

./dd2.f90:5:24: warning: EXTERNAL attribute was already specified on 'f' [-Wredundant-attribute]
              subroutine f()
                         ^

@klausler
Copy link
Contributor Author

klausler commented Sep 8, 2025

@kkwli both of the examples in your comments above should now be warnings. I'll check them and update the patch if necessary

UPDATE: Both of your examples above now produce only warnings with my patch. Are you seeing something different, @kkwli?

@kkwli
Copy link
Collaborator

kkwli commented Sep 8, 2025

@kkwli both of the examples in your comments above should now be warnings. I'll check them and update the patch if necessary

UPDATE: Both of your examples above now produce only warnings with my patch. Are you seeing something different, @kkwli?

@klausler My bad! I messed up the build! A warning message is emitted in both cases. Everything looks fine.

@klausler
Copy link
Contributor Author

klausler commented Sep 8, 2025

@kkwli both of the examples in your comments above should now be warnings. I'll check them and update the patch if necessary
UPDATE: Both of your examples above now produce only warnings with my patch. Are you seeing something different, @kkwli?

@klausler My bad! I messed up the build! A warning message is emitted in both cases. Everything looks fine.

Thanks for checking.

@klausler klausler enabled auto-merge (squash) September 10, 2025 20:15
@klausler klausler disabled auto-merge September 10, 2025 20:38
@klausler klausler merged commit e062b9c into llvm:main Sep 10, 2025
5 of 9 checks passed
@klausler klausler deleted the bug157162 branch September 10, 2025 20:38
@kparzysz
Copy link
Contributor

What happened to the CI checks in this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[flang] inconsistent message for duplicate external procedure declaration
5 participants